[DataGrid] Add missing slotProps to panel interface#22651
Conversation
cherniavskii
added
type: bug
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
| } | ||
|
|
||
| export default function CustomFilterPanelPosition() { | ||
| export default function CustomFilterPanelPositionNoSnap() { |
There was a problem hiding this comment.
Since this demo customizes the panel placement that is not visible when the screenshot is taken, I've excluded it from being screenshoted.
siriwatknp
left a comment
siriwatknp
left a comment
There was a problem hiding this comment.
👍 Thanks for taking care of this, here is what I found:
- The switch to
Omit<GridSlotProps['basePopper'], 'ref'>also exposes the internalon*handlers as settable, which can break the panel. Suggest keepingPickand only addingplacement+ safe booleans likefocusTrap. open: booleanre-declaration inGridPanelPropsis now redundant, sincePopperPropsalready requires it.
| GridSlotProps['basePopper'], | ||
| 'id' | 'className' | 'target' | 'flip' | ||
| > { | ||
| export interface GridPanelProps extends Omit<GridSlotProps['basePopper'], 'ref'> { |
There was a problem hiding this comment.
(1) This widening exposes the on* handlers (onDidShow, onDidHide, onClickAway, and the clickAway*Event props) through slotProps.panel, but they are controlled internally.
The {...other} spread runs after onDidShow={onDidShow}, so a consumer override stops setIsPlaced(true) from firing. Then {isPlaced && children} never renders the panel content. Same path breaks onClickAway and onDidHide.
I think it is safer to keep the existing Pick and only add props with a clear use case. placement is the real motivation here, and boolean toggles like focusTrap are safe because they cannot break behavior.
Could be something like this:
Pick<
GridSlotProps['basePopper'],
'id' | 'className' | 'target' | 'flip' | 'placement' | 'focusTrap'
>For the on* handlers, maybe hold off for now. They are risky without a real example or doc showing how to compose them with the internal handlers. Keeping the fix small avoids advertising a footgun.
| GridSlotProps['basePopper'], | ||
| 'id' | 'className' | 'target' | 'flip' | ||
| > { | ||
| export interface GridPanelProps extends Omit<GridSlotProps['basePopper'], 'ref'> { |
There was a problem hiding this comment.
(2) Minor: with this Omit, GridPanelProps now inherits a required open: boolean from PopperProps (gridBaseSlots.ts:211). So the open: boolean re-declaration on line 33 is identical and dead. Please drop that line.
onClose is not in PopperProps, so keep it.
- open: boolean;
onClose?: () => void;2 participants
While creating a customization recipe for a support request, I noticed that some props are being passed in runtime but are missing in TS interface (e.g.
placement).I've updated the interface to reflect what is actually being passed in runtime.
I've also added a customization demo that mimics the default v7 behavior (this is what the support request was about).
Preview: https://deploy-preview-22651--material-ui-x.netlify.app/x/react-data-grid/filtering/customization/#customize-the-filter-panel-position